Skip to content

Add a delay after rtc_set_datetime #2156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 6, 2025
Merged

Add a delay after rtc_set_datetime #2156

merged 1 commit into from
Feb 6, 2025

Conversation

@peterharperuk
Copy link
Contributor

If you call rtc_get_datetime immediately after rtc_set_datetime you get junk back. According to the datasheet "Writing to the RTC will take 2 clk_rtc clock periods to arrive". So add a delay after calling rtc_set_datetime in aon_timer_set_time_calendar.

Fixes #2148

lurch
lurch reviewed Jan 3, 2025
View reviewed changes
@lurch
Copy link
Contributor

lurch commented Jan 3, 2025

ping @armandomontanez - any idea why the CMake build is passing but the Bazel build is failing?

@armandomontanez
Copy link
Contributor

armandomontanez commented Jan 5, 2025 •
edited
Loading

Looks like you need to add //src/rp2_common/pico_aon_timer to the deps of the test. You also might need to add //src/common/pico_time to the deps of pico_aon_timer since it's pulling in a new header. If you hit another snag let me know and I'll cook up a diff to get it working.

@peterharperuk peterharperuk added this to the 2.1.1 milestone Jan 29, 2025
kilograham
kilograham reviewed Feb 2, 2025
View reviewed changes
kilograham
kilograham previously approved these changes Feb 3, 2025
View reviewed changes
@kilograham
Copy link
Contributor

@peterharperuk can you fix merge conflicts and i'll re-review

@peterharperuk peterharperuk force-pushed the rtc_set_then_get_with_aon branch 2 times, most recently from 7249c74 to f26fe7a Compare February 4, 2025 12:05
@peterharperuk
Copy link
Contributor Author

Apologies. Merge conflicts fixed. And I squashed everything together.

@kilograham
Copy link
Contributor

Ah, not sure if this is fixed in different issue but you have added #include "pico/aon_timer.h" to pico_time_test.c which will break host build; it (and test which use it) should be guarded by #if LIB_PICO_AON_TIMER probably

pico_aon_timer now depends on hardware_clocks AND pico_time.. probably should be explicit about dependencies (note just pico_time_headers is likely fine. Note in the CMakeLists.txt I'd also make the dependency (and the includes) RP2040 only

@peterharperuk peterharperuk force-pushed the rtc_set_then_get_with_aon branch from f26fe7a to dce3159 Compare February 5, 2025 11:01
@peterharperuk
Copy link
Contributor Author

I've tried to make the changes you suggested. You'll have to tell me how to test aon timer in a host build.

@peterharperuk peterharperuk force-pushed the rtc_set_then_get_with_aon branch from dce3159 to c3a3e12 Compare February 5, 2025 11:19
@peterharperuk
Add a delay after rtc_set_datetime
f32bfca 
If you call rtc_get_datetime immediately after rtc_set_datetime you get
junk back. According to the datasheet "Writing to the RTC will take
2 clk_rtc clock periods to arrive". So add a delay after calling
rtc_set_datetime in aon_timer_set_time_calendar.

Fixes raspberrypi#2148
@peterharperuk peterharperuk force-pushed the rtc_set_then_get_with_aon branch from c3a3e12 to f32bfca Compare February 5, 2025 12:00
@kilograham
Copy link
Contributor

cmake -DPICO_PLATFORM=host .. then build everything

kilograham
kilograham reviewed Feb 5, 2025
View reviewed changes
@kilograham kilograham merged commit 7dfbb3c into raspberrypi:develop Feb 6, 2025
4 checks passed
will-v-pi pushed a commit to will-v-pi/pico-sdk that referenced this pull request Mar 20, 2025
@peterharperuk @will-v-pi
Add a delay after rtc_set_datetime (raspberrypi#2156)
c2e4044 
If you call rtc_get_datetime immediately after rtc_set_datetime you get
junk back. According to the datasheet "Writing to the RTC will take
2 clk_rtc clock periods to arrive". So add a delay after calling
rtc_set_datetime in aon_timer_set_time_calendar.

Fixes raspberrypi#2148
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lurch lurch lurch left review comments

@kilograham kilograham kilograham approved these changes

Assignees

@kilograham kilograham

Labels

None yet

Projects

None yet

Milestone

2.1.1

Development

Successfully merging this pull request may close these issues.

4 participants

@peterharperuk @lurch @armandomontanez @kilograham